-
Notifications
You must be signed in to change notification settings - Fork 234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove Support for Databricks 10.4 [databricks] #10474
Conversation
Signed-off-by: Tim Liu <[email protected]>
Signed-off-by: Tim Liu <[email protected]>
Signed-off-by: Raza Jafri <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are other references to 321db that have been missed. git grep 321db
will show where they are, e.g.: profiles in aggregator/pom.xml, delta-spark321db project should be removed.
...gen/src/main/spark320/scala/org/apache/spark/sql/tests/datagen/datagen/DataGenExprBase.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/spark311/scala/org/apache/spark/sql/rapids/shims/GpuAscii.scala
Outdated
Show resolved
Hide resolved
@@ -1,6 +1,6 @@ | |||
#!/usr/local/env groovy | |||
/* | |||
* Copyright (c) 2023, NVIDIA CORPORATION. | |||
* Copyright (c) 2023-2024, NVIDIA CORPORATION. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check copyrights in every file. Since you haven't used the pre-commit, the easiest is probably to run
./scripts/auto-copyrighter.sh $(git diff --name-only origin/branch-24.04..HEAD)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! My pre-commit is broken somehow and I assumed it ran.
This should've been a draft PR PTAL again |
build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, one optional comment
@@ -24,7 +24,7 @@ import org.apache.spark.sql.rapids.shims.ArrowUtilsShim | |||
import org.apache.spark.sql.types._ | |||
import org.apache.spark.sql.vectorized.ColumnarBatch | |||
|
|||
//TODO is this needed? we already have a similar version in spark321db | |||
//TODO is this needed? we already have a similar version in spark330db |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you'll have to run CI another time, please drop this obsolete comment beforehand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, +1
This PR drops support for Databricsk 10.4
Changes:
mvn generate-sources -Dshimplify=true -Dshimplify.move=true -Dshimplify.remove.shim=321db
, fixed up some unnecessary empty lines that were introduced.Tests:
fixes #10429